[java-generator] Overridable package name #4694

andreaTP · 2022-12-19T14:35:04Z

Description

Fix #4621

Type of change

Bug fix (non-breaking change which fixes an issue)
Feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change
Chore (non-breaking change which doesn't affect codebase;
test, version modification, documentation, etc.)

Checklist

Code contributed by me aligns with current project license: Apache 2.0
I Added CHANGELOG entry regarding this change
I have implemented unit tests to cover my changes
I have added/updated the javadocs and other documentation accordingly
No new bugs, code smells, etc. in SonarCloud report
I tested my code in Kubernetes
I tested my code in OpenShift

rohanKanojia · 2022-12-22T07:09:45Z

@andreaTP : Do you think we should add documentation for this new flag in doc/java-generation-from-CRD.md

andreaTP · 2022-12-22T11:05:47Z

@rohanKanojia good call!
We should re-generate the documentation from CLI and Maven: https://github.com/fabric8io/kubernetes-client/blob/master/doc/java-generation-from-CRD.md#usage
Tracked here: 4707 since I have a few improvements in the pipeline and I would like to do it once at the end to avoid continuous git conflicts.

manusa · 2022-12-23T06:08:30Z

CHANGELOG.md

+### 6.4.0
+
+#### New features
+* Fix #4694: [java-generator] Option to override the package name of the generated code.
+


This should be moved to the SNAPSHOT section, I guess that since we released 6.3.1 while you were working on this you didn't notice.

manusa

Sorry for the belated review.

Besides the next boring section, I think it would be good to update the docs too (however, you created another more generic issue for that).

While I like the solution, I'm not completely convinced about its intuitiveness.

As I see it, we're offering the possibility to users to map the final computed, conventional packages to a different name. However, this is under the assumption that the user is aware of what the final package name will be according to our internal convention (and this is were I'm probably making wrong assumptions).

So basically we're telling the user to configure the final step in the following flow:
gorup -> convention.package -> custom.package

I insist that maybe users would be more comfortable with what you proposed here and this is just to raise a point and a discussion. Wouldn't it be more intuitive for users to map the group/name to a package name:

<broker.amq.io>com.example.crd.amq</broker.amq.io>

Although now that I'm seeing this, my proposal has the problem of how to include the version (maybe nested, but this would be ugly).

Anyway, I'd just like to know your thoughts on this before probably merging it as it is. Otherwise, it's pretty cool ;) 🙌

andreaTP · 2022-12-23T10:26:33Z

@manusa you are right that the current implementation is not great in terms of usability, but I cannot find a better implementation of it 🙁

The reasoning to be applied here is the following:

integrate the generator into your project
you are not satisfied with one or more of the package names
you get the current and apply the override in the configuration

Matching on the plain group would have been slightly better, but you have then the problem with the version and everything becomes messy again.

Possible improvements over this might be to log the package names and provide the user with some information in the terminal ... but, again this is not going to be great ...